Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Conditionally enable profiler component stacks #19396

Merged
merged 3 commits into from
Jul 17, 2020
Merged

Conditionally enable profiler component stacks #19396

merged 3 commits into from
Jul 17, 2020

Conversation

bvaughn
Copy link
Contributor

@bvaughn bvaughn commented Jul 17, 2020 •
edited
Loading

This PR stacks on top of #19376 and includes only one commit, e6e3d8b

The new component stack approach introduced in #18495 seems to cause bugs (e.g. 1, 2) when used by the new profiler marks (#19376), so this PR moves that part of the profiler behind a feature flag (off by default).

This will enable us to sync the new profiler marks to the new reconciler and let people start using them within Facebook safely, while still enable me to further investigate why the errors are happening. (I've done a www sync of this PR and verified that it fixes the 300 and 310 errors I've been seeing on Facebook.com)

Note that this is a dynamic feature flag, but it's only used once during initialization so it should be okay.

@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Jul 17, 2020
@codesandbox-ci
Copy link

codesandbox-ci bot commented Jul 17, 2020 •
edited
Loading

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit e6e3d8b:

Sandbox Source
React Configuration

@sizebot
Copy link

sizebot commented Jul 17, 2020 •
edited
Loading

Details of bundled changes.

Comparing: aec934a...e6e3d8b

react-dom

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-dom.development.js +0.5% +0.4% 913.5 KB 918 KB 207.61 KB 208.43 KB NODE_DEV
react-dom.production.min.js 0.0% 0.0% 123.77 KB 123.77 KB 39.64 KB 39.64 KB NODE_PROD
ReactDOMForked-profiling.js +0.9% +1.2% 405.46 KB 409.23 KB 75.27 KB 76.17 KB FB_WWW_PROFILING
react-dom-test-utils.production.min.js 0.0% 0.0% 10.35 KB 10.35 KB 4.07 KB 4.07 KB UMD_PROD
ReactDOMTesting-dev.js -0.0% -0.0% 953.4 KB 953.4 KB 213.62 KB 213.62 KB FB_WWW_DEV
react-dom-test-utils.development.js 0.0% 0.0% 51.15 KB 51.15 KB 14.9 KB 14.9 KB NODE_DEV
react-dom-unstable-fizz.node.development.js 0.0% +0.1% 5.61 KB 5.61 KB 1.86 KB 1.86 KB NODE_DEV
react-dom-test-utils.production.min.js 0.0% 0.0% 10.2 KB 10.2 KB 3.99 KB 3.99 KB NODE_PROD
react-dom-unstable-fizz.browser.development.js 0.0% +0.1% 5.36 KB 5.36 KB 1.8 KB 1.8 KB UMD_DEV
react-dom-unstable-fizz.node.production.min.js 0.0% -0.3% 1.17 KB 1.17 KB 667 B 665 B NODE_PROD
ReactTestUtils-dev.js +0.1% 0.0% 45.25 KB 45.29 KB 12.74 KB 12.74 KB FB_WWW_DEV
react-dom.development.js +0.5% +0.4% 959.51 KB 964.37 KB 210.14 KB 210.99 KB UMD_DEV
react-dom.profiling.min.js +1.0% +1.0% 127.53 KB 128.78 KB 41.66 KB 42.06 KB UMD_PROFILING
ReactDOMForked-dev.js +1.2% +1.0% 979.15 KB 991.17 KB 218.61 KB 220.75 KB FB_WWW_DEV
react-dom.profiling.min.js +1.0% +1.0% 127.88 KB 129.14 KB 40.92 KB 41.32 KB NODE_PROFILING
ReactDOM-dev.js -0.1% -0.0% 988.54 KB 987.81 KB 220.03 KB 220.01 KB FB_WWW_DEV
ReactDOM-prod.js -1.2% -1.3% 398.22 KB 393.48 KB 74.17 KB 73.21 KB FB_WWW_PROD
ReactDOM-profiling.js -0.2% -0.1% 408.85 KB 407.92 KB 75.92 KB 75.84 KB FB_WWW_PROFILING
ReactDOMServer-dev.js 0.0% 0.0% 143.3 KB 143.33 KB 36.42 KB 36.43 KB FB_WWW_DEV

react-art

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
ReactART-prod.js -2.0% -2.2% 244.7 KB 239.91 KB 43.4 KB 42.46 KB FB_WWW_PROD
react-art.development.js +0.7% +0.6% 689.21 KB 694.07 KB 146.06 KB 146.92 KB UMD_DEV
react-art.development.js +0.8% +0.6% 590.64 KB 595.15 KB 128.25 KB 129.08 KB NODE_DEV
ReactART-dev.js -0.1% -0.0% 627.88 KB 627.16 KB 133.28 KB 133.25 KB FB_WWW_DEV

react-reconciler

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-reconciler.development.js +0.7% +0.6% 650.38 KB 654.89 KB 138.62 KB 139.45 KB NODE_DEV
react-reconciler.production.min.js 0.0% 0.0% 87.6 KB 87.6 KB 26.85 KB 26.85 KB NODE_PROD

ReactDOM: size: 0.0%, gzip: 0.0%

Size changes (experimental)

Generated by 🚫 dangerJS against e6e3d8b

@sizebot
Copy link

sizebot commented Jul 17, 2020 •
edited
Loading

Details of bundled changes.

Comparing: aec934a...e6e3d8b

react-art

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
ReactART-prod.js -1.9% -2.1% 251.84 KB 247.06 KB 44.68 KB 43.73 KB FB_WWW_PROD
react-art.development.js -0.0% -0.0% 666.81 KB 666.81 KB 141.91 KB 141.91 KB UMD_DEV
react-art.production.min.js 0.0% 0.0% 109.63 KB 109.63 KB 33.97 KB 33.97 KB UMD_PROD
react-art.development.js -0.0% -0.0% 569.16 KB 569.16 KB 124.16 KB 124.15 KB NODE_DEV
ReactART-dev.js -0.1% -0.0% 637.9 KB 637.17 KB 135.31 KB 135.29 KB FB_WWW_DEV

react-dom

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-dom.development.js -0.0% 0.0% 880.96 KB 880.96 KB 201.72 KB 201.72 KB NODE_DEV
ReactDOMForked-prod.js 0.0% 0.0% 405.39 KB 405.39 KB 75.02 KB 75.02 KB FB_WWW_PROD
react-dom-server.node.development.js 0.0% 0.0% 137.33 KB 137.33 KB 36.49 KB 36.49 KB NODE_DEV
react-dom.production.min.js 0.0% 0.0% 119.01 KB 119.01 KB 38.24 KB 38.24 KB NODE_PROD
ReactDOMForked-profiling.js +0.9% +1.1% 416.65 KB 420.42 KB 76.81 KB 77.68 KB FB_WWW_PROFILING
react-dom-test-utils.production.min.js 0.0% 0.0% 10.34 KB 10.34 KB 4.06 KB 4.06 KB UMD_PROD
ReactDOMTesting-dev.js -0.0% -0.0% 981.48 KB 981.48 KB 219.51 KB 219.51 KB FB_WWW_DEV
react-dom-test-utils.development.js 0.0% 0.0% 51.13 KB 51.13 KB 14.89 KB 14.89 KB NODE_DEV
react-dom-test-utils.production.min.js 0.0% 0.0% 10.19 KB 10.19 KB 3.98 KB 3.98 KB NODE_PROD
ReactTestUtils-dev.js +0.1% +0.1% 45.25 KB 45.29 KB 12.73 KB 12.74 KB FB_WWW_DEV
react-dom.development.js -0.0% -0.0% 925.53 KB 925.53 KB 204.14 KB 204.14 KB UMD_DEV
react-dom.production.min.js 0.0% 0.0% 118.89 KB 118.89 KB 39.04 KB 39.04 KB UMD_PROD
react-dom.profiling.min.js 0.0% 0.0% 122.8 KB 122.8 KB 40.18 KB 40.18 KB UMD_PROFILING
ReactDOMForked-dev.js +1.2% +1.0% 1004.69 KB 1016.71 KB 223.81 KB 225.95 KB FB_WWW_DEV
react-dom.profiling.min.js 0.0% 0.0% 123.07 KB 123.07 KB 39.45 KB 39.45 KB NODE_PROFILING
react-dom-server.browser.production.min.js 0.0% -0.0% 19.92 KB 19.92 KB 7.46 KB 7.46 KB UMD_PROD
ReactDOM-dev.js -0.1% -0.0% 1014.08 KB 1013.36 KB 225.23 KB 225.21 KB FB_WWW_DEV
ReactDOM-prod.js -1.2% -1.3% 409.36 KB 404.62 KB 75.7 KB 74.73 KB FB_WWW_PROD
react-dom-server.browser.development.js 0.0% 0.0% 136.06 KB 136.06 KB 36.24 KB 36.24 KB NODE_DEV
ReactDOM-profiling.js -0.2% -0.1% 420.04 KB 419.12 KB 77.46 KB 77.37 KB FB_WWW_PROFILING
react-dom-server.browser.production.min.js 0.0% -0.0% 19.84 KB 19.84 KB 7.44 KB 7.44 KB NODE_PROD
ReactDOMServer-dev.js 0.0% 0.0% 147.33 KB 147.37 KB 37.42 KB 37.43 KB FB_WWW_DEV
ReactDOMServer-prod.js 0.0% 0.0% 47.54 KB 47.54 KB 11.14 KB 11.14 KB FB_WWW_PROD

ReactDOM: size: 0.0%, gzip: 0.0%

Size changes (stable)

Generated by 🚫 dangerJS against e6e3d8b

@bvaughn bvaughn force-pushed the conditionally-enable-profiler-component-stacks branch from 2eb9a5d to 2d15507 Compare July 17, 2020 13:53
@bvaughn bvaughn force-pushed the conditionally-enable-profiler-component-stacks branch from 2d15507 to e6e3d8b Compare July 17, 2020 13:56
@bvaughn
Copy link
Contributor Author

bvaughn commented Jul 17, 2020

This PR fixes the 300 and 310 errors I've been seeing on Facebook.com (tested with an on-demand just now).

trueadm
trueadm approved these changes Jul 17, 2020
View reviewed changes
Copy link
Contributor

@trueadm trueadm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@bvaughn bvaughn merged commit 51267c4 into facebook:master Jul 17, 2020
@bvaughn bvaughn deleted the conditionally-enable-profiler-component-stacks branch July 17, 2020 15:24
This was referenced Jul 17, 2020
Closed
Closed
@taneliang taneliang mentioned this pull request Jul 21, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@trueadm trueadm trueadm approved these changes

@sebmarkbage sebmarkbage Awaiting requested review from sebmarkbage

@gaearon gaearon Awaiting requested review from gaearon

@rickhanlonii rickhanlonii Awaiting requested review from rickhanlonii

@lunaruan lunaruan Awaiting requested review from lunaruan

@acdlite acdlite Awaiting requested review from acdlite

Assignees
No one assigned
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@bvaughn @sizebot @trueadm @facebook-github-bot